test(vm_disk): cover non-default tiering_priority_factor (regression coverage for #30) - #377
Open
ddemlow wants to merge 1 commit into
Open
test(vm_disk): cover non-default tiering_priority_factor (regression coverage for #30)#377ddemlow wants to merge 1 commit into
ddemlow wants to merge 1 commit into
Conversation
Every pre-existing tiering_priority_factor assertion in this target checks the DEFAULT (4), in all 8 places, so none of them could detect the #30 behaviour: HyperCore ignoring tieringPriorityFactor on disk CREATE and forcing the default, requiring a second pass (internal Scale REST 5143). Adds coverage that a non-default factor lands on the FIRST pass, that a repeat is idempotent, and that changing it on an existing disk takes effect. Verified passing against HyperCore 9.7.7.226383 - the create-path bug is fixed upstream; the collection never carried a workaround. The change-path assertion uses retries because on 9.7.7 a GET immediately after the change still returns the OLD factor for a few seconds, even though _update_block_device already waits on the TaskTag and HyperCore reports it COMPLETE. That read-after-write lag is distinct from #30.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds integration coverage for a non-default
tiering_priority_factor, closing the gap that let #30 go undetected for ~4 years.Why
Every pre-existing
tiering_priority_factorassertion intests/integration/targets/vm_disk/tasks/main.ymlchecks the default value —== 4, in all 8 places it appears — and nothing in the target ever sets a non-default value.That means the suite could not observe #30 in either direction. The bug ("HyperCore ignores
tieringPriorityFactoron disk create and forces the default, so it takes two passes") existed, sat for years, was fixed upstream, andvm_diskreported pass throughout.#30 has now been verified fixed on HyperCore 9.7.7.226383 and closed. This adds the coverage so a re-regression is caught.
What it covers
vm_info).changed=false) — the original bug made it non-idempotent.The VM has no disks at the point this runs, so the block creates its own disk and force-removes it afterwards — no new VM, and no interference with the existing assertions.
One implementation note worth reviewing
The assertion in (4) uses
retries/untilrather than a bare assert. On 9.7.7, changing tiering on an existing disk is accepted but not visible to a GET for a few seconds — the module returnschanged=truewhile its own returnedrecord, andvm_info, still carry the old factor. It settles well within 60s.This is not a missing wait in the collection:
ManageVMDisks._update_block_devicealready callsTaskTag.wait_task(plugins/module_utils/vm.py:1286), and HyperCore reports the task COMPLETE before the new value is readable — so it looks like eventual consistency on the HyperCore side.A bare assert immediately after the change fails intermittently; that is what the retry is there for, and there is a comment in the test explaining it. Flagging it explicitly in case you would rather track that lag separately.
Verification
Ran against HyperCore 9.7.7.226383 (4-node cluster), ansible-core 2.16.19:
Passes, no failures. Runtime goes from ~108s to ~149s (+41s), most of which is the settle poll in (4).
Notes
changelogs/fragments/entry added, since no existing fragment covers a test-only change and this has no user-facing effect. Happy to add one if you would prefer.integ-test.ymlisschedule+workflow_dispatchonly), so this will not be exercised by PR CI — it was verified locally as above, and will be picked up by the nightly run once merged.